fix: do not inject into commented-out head/body/html tags#34283
Open
chang-pro wants to merge 2 commits into
Open
fix: do not inject into commented-out head/body/html tags#34283chang-pro wants to merge 2 commits into
chang-pro wants to merge 2 commits into
Conversation
The proxy html rewriter picked its injection point with regexes over the raw response body, so a <head> (or <body>/<html>/<!doctype>) inside an HTML comment became the injection target and the served page got corrupted. Search a comment-masked copy of the html instead and splice into the original by index. Fixes cypress-io#33000
chang-pro
requested a deployment
to
fork-pr-review
GitHub Actions
Waiting
chang-pro
requested a deployment
to
fork-pr-review
GitHub Actions
Waiting
chang-pro
requested a deployment
to
fork-pr-review
GitHub Actions
Waiting
|
|
Collaborator
|
…masking quote-aware The data-cy-bootstrap search ran before comment masking, so a commented-out bootstrap script swallowed the injection. The comment masker also treated <!-- inside a quoted attribute value as a comment opener and did not recognize abrupt (<!-->) or --!> closings, blanking the rest of the document and dropping real head attributes. Masking now uses a small tag/quote-aware scanner and the bootstrap search runs on the masked copy, splicing the original html by index.
chang-pro
requested a deployment
to
fork-pr-review
GitHub Actions
Waiting
chang-pro
requested a deployment
to
fork-pr-review
GitHub Actions
Waiting
chang-pro
requested a deployment
to
fork-pr-review
GitHub Actions
Waiting
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
packages/proxy/lib/http/util/rewriter.tsfrom working properly #33000Additional details
The proxy's html rewriter picks its injection point by running
headRe/bodyRe/htmlRe/doctypeReover the raw response body, so a<head>inside an HTML comment becomes the injection target: the Cypress script lands inside the comment and a stray real<head></head>gets emitted, corrupting the page.Instead of trying to make each regex comment-aware (the direction previous attempts took), the rewriter now masks comment ranges with same-length whitespace, matches against the masked copy, and splices into the original string by index (the insert helpers only use
match.index/match[0].length, which stay aligned). An unterminated<!--masks to the end of the document, matching how browsers treat it. Thedata-cy-bootstrappath is unchanged.Steps to test
Run the spec from the issue:
The comment now survives untouched and the injection lands in a real
<head>placed after<html>. Unit coverage:packages/proxy/test/unit/http/util/rewriter.spec.ts(3 new specs covering commented head, commented body before a real body, and an unterminated comment; all 3 fail without the fix, 6/6 pass with it).How has the user experience changed?
Pages containing commented-out
<head>/<body>/<html>tags are no longer corrupted by the proxy. Before/after of the served HTML for the issue's repro:Before:
<html><!-- <head> <script>CYPRESS_INJECTION()</script><title>Test</title></head> --><head> </head></html>After:
<html> <head> <script>CYPRESS_INJECTION()</script> </head><!-- <head><title>Test</title></head> --></html>PR Tasks
cypress-documentation? N/Atype definitions? N/Acypress.schema.json? N/A